Skip to content

Conversation

@callu9
Copy link
Collaborator

@callu9 callu9 commented Jan 24, 2025

04. 스프린트 미션 4

요구사항

스프린트 미션 4 시안

기본 요구사항

체크리스트 [기본]

로그인

  • 이메일 input에서 focus out 할 때, 값이 없을 경우 input에 빨강색 테두리와 아래에 “이메일을 입력해주세요.” 빨강색 에러 메세지를 보입니다.

  • 이메일 input에서 focus out 할 때, 이메일 형식에 맞지 않는 경우 input에 빨강색 테두리와 아래에 “잘못된 이메일 형식입니다” 빨강색 에러 메세지를 보입니다.

  • 비밀번호 input에서 focus out 할 때, 값이 없을 경우 아래에 “비밀번호를 입력해주세요.” 에러 메세지를 보입니다

  • 비밀번호 input에서 focus out 할 때, 값이 8자 미만일 경우 아래에 “비밀번호를 8자 이상 입력해주세요.” 에러 메세지를 보입니다.

  • input 에 빈 값이 있거나 에러 메세지가 있으면 ‘로그인’ 버튼은 비활성화 됩니다.

  • Input 에 유효한 값을 입력하면 ‘로그인' 버튼이 활성화 됩니다.

  • 활성화된 ‘로그인’ 버튼을 누르면 “/items” 로 이동합니다

회원가입

체크리스트 [심화]

  • 눈 모양 아이콘 클릭시 비밀번호의 문자열이 보이기도 하고, 가려지기도 합니다.
  • 비밀번호의 문자열이 가려질 때는 눈 모양 아이콘에는 사선이 그어져있고, 비밀번호의 문자열이 보일 때는 사선이 없는 눈 모양 아이콘이 보이도록 합니다.

주요 변경사항

스프린트 미션 3 리뷰 반영

  • login.js, signup.js
    • 변수 지정 및 함수명 변경을 통한 가독성 확보
  • login.html, signup.html
    • <p> 태그는 문단을 나타낼 때만 사용으로 변경
    • 비밀번호 확인 <input type="checkbox" />에서 <button type="button" />사용으로 변경
  • login.css, signup.css
    • signup.css의 중복되는 스타일 제거

스프린트 미션 4

  • members.js 추가를 통한 공통 함수 분리

스크린샷

로그인

  • 에러문구 표시, 비밀번호 입력 보기, 로그인 버튼 활성화, 페이지 이동
    longin-gif-login-page

회원가입

  • 에러문구 표시, 회원가입 버튼 활성화, 페이지 이동
    signup-gif-signup-page

멘토에게

  • <script type="module" src="./scripts/login.js"></script>과 같이 type="module" 속성을 주어야만 login.js 파일에서 /common/members.js 파일을 불러올 수 있는 것으로 보입니다. 해당 속성을 사용해야만 하는 이유가 궁금합니다.
  • 공통함수 members.js가 코드량이 많다보니, 더 효율적이고 가독성 좋게 바꿀 수 있는 방법이 있을지 궁금합니다.
  • login.js, signup.js 두 파일이 비슷하게 생긴 점, signup.html에서 두 스크립트 파일을 불러오는 점에 개선점이 있을지 궁금합니다.

@callu9 callu9 requested a review from dongqui January 24, 2025 00:41
@callu9 callu9 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label Jan 24, 2025
Copy link
Collaborator

@dongqui dongqui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정님~! 이번 미션도 멋지게 마무리 하셨네요 💯

코드를 구조화하시려는 모습이 보기 좋습니다!! dom 접근과 관련해 조금 더 명시적인 방법을 사용하면 좋을 거 같습니다 :)

<script type="module" src="./scripts/login.js"></script>과 같이 type="module" 속성을 주어야만 login.js 파일에서 /common/members.js 파일을 불러올 수 있는 것으로 보입니다. 해당 속성을 사용해야만 하는 이유가 궁금합니다.
-> 모듈은 비교적 최근에 나온 문법입니다! 본래는 모든 자바스크립트 파일이 전역 스코프에서 실행되어 왔어요. 모듈 환경이 지원되기 시작했고, 브라우저에게 해당 파일을 모듈로 해석하라고 명시해주는 것이 말씀하신 속성의 역할입니다 :)

공통함수 members.js가 코드량이 많다보니, 더 효율적이고 가독성 좋게 바꿀 수 있는 방법이 있을지 궁금합니다.
-> 함수나 변수 이름을 잘 지어주시고 추상화를 잘 해주시면 가독성에 많은 도움이 됩니다
. 추상화 관련해서는 리뷰에도 남겨드렸어요 :)
그리고 하나의 함수는 하나의 일을 하는 것이 이상적입니다! (언제나 그럴 수는 없지만요)
정의 해주신 checkValidation을 예로 들면 유효성 검사도 하고 에러 UI도 핸들링하고 있죠. checkValidation을 사용할 때 UI까지 수정하고 있다는 것을 알 수 있는 이름으로 지어주시거나, 로직을 분리하시는 게 좋을 거 같네요! 또한 모든 유효성 검사를 한 번에 하고 있다보니, 추가 사항이 생길 때 마다 함수가 비대해지고 재사용이 힘들어지는 구조를 가지고 있습니다. 🤔

login.js, signup.js 두 파일이 비슷하게 생긴 점, signup.html에서 두 스크립트 파일을 불러오는 점에 개선점이 있을지 궁금합니다.
-> 지금도 충분히 잘하셨습니다 :) 위 말씀드린대로 함수를 작게 쪼개고, 공통된 부분을 모아 구조화하면 재사용성을 늘릴 수 있습니다! 다만, 지금은 자바스크립트 개념 학습에 충실하셔도 충분하다고 생각합니다. 아래 세 가지 정도만 조금씩 실천해주세요~! :)

  1. 함수, 변수명 잘 짓기
  2. 단일 책임(되로록 함수는 하나의 일을 하도록)
  3. 함수로 코드 가독성 높이기

export function onVisibilityChange(e) {
if (e.target.localName !== "button") return;
const button = e.target;
const input = document.getElementById(button.id).previousElementSibling;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

비밀번호 숨기는 로직을 하나의 함수로 처리 하셨군요! 👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

html dataset을 이용해서 input id를 가져오는 방법도 있겠네요~! (참고만 해주세요!)

return;
}

const inputField = input.parentElement?.parentElement;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 부분들은 가독성을 위해 함수로 추상화 하시면 좋습니다~!
handleErrorMessage 같은 함수 내에 해당 로직을 정리해놓으면, 함수 이름만 보고 여기서 에러 메세지를 다루는구나~ 하고 이해할 수 있지만 지금의 경우는 한 줄 한 줄 읽어야하죠 :)

나중에 에러 메세지 관련된 부분을 이해해야 하거나 수정해야 할 때는 해당 함수만 찾아가면 되구요!

const inputField = input.parentElement?.parentElement;
const errorParagraph = document.getElementsByClassName(className)[0];

if (result) errorParagraph && inputField?.removeChild(errorParagraph);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

약간은 복잡한 느낌이 있는데,

<span id="email-error"></span>

미리 html에 정의하고 display, content만 수정할 수도 있을 거 같네요!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

참고만 해주세요!

let isEmptyInput = false;
const form = document.getElementsByTagName("form")[0];
for (let inputField of form.children) {
if (!inputField.classList.value.includes("input-field")) break;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위와 마찬가지 입니다 :) 추상화로 가독성을 높여주세요!

default:
return;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previousElementSibling, parentsElement 등을 사용해서 dom 접근하는 코드가 많이 보이는데 dataset, id 등을 좀 더 활용해 보셔도 좋을 거 같습니다 :)

기존에 사용하고 계시는 dom 접근법은 간단하게 사용하면 괜찮지만, 갈수록 유지 보수가 어려워집니다!
HTML 구조에 따라 코드가 쉽게 깨지는 문제가 있고 html 구조를 같이 봐야 하기 때문에 코드를 이해하기가 힘들어지죠 😢

우선 참고만 해주세요~!

@dongqui
Copy link
Collaborator

dongqui commented Jan 24, 2025

참고 코드 남겨드립니다. 공통된 부분을 구조화해서 재사용성을 높여 필요한 부분만 import 할 수 있습니다.
모든 코드가 그렇듯 정답은 아닙니다!

모든 로직을 이해하실 필요도 없고, 이렇게도 하는구나~하고 대충 보시면 될 거 같아요! 🤣

// -------- members.js -------
function validateAndUpdateUI() {
  const value = this.$element.value;

  this.error = this.validations.find((validation) => {
    return !validation.test(value);
  })?.message;

  this.error
    ? this.$element.classList.add("error")
    : this.$element.classList.remove("error");

  this.$errorElement.innerHTML = this.error || "";
}

export const email = {
  $element: document.getElementById("email"),
  $errorElement: document.getElementById("email-error"),
  validations: [
    {
      test: (value) => !!email,
      message: "비어있습니다",
    },
    {
      test: (value) =>
        /^[A-Za-z0-9._%-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4}$/.test(email),
      message: "잘못된 이메일입니다",
    },
  ],
  error: null,
  isDirty: false,
  validateAndUpdateUI,
};

export const password = {
  $element: document.getElementById("password"),
  $errorElement: document.getElementById("password-error"),
  validations: [
   {
      test: (value) => value.length === 0,
      message: "비었음",
    },
    {
      test: (value) => value.length >= 8,
      message: "8글자 이상 ",
    },
  ],
  error: null,
  isDirty: false,
  validateAndUpdateUI,
};

//... 중략

export const updateSubmitButtonState = (formElements) => {
  const submitButton = document.querySelector("#submit");
  const formValid = Object.values(formElements).every(
    (element) => !element.error && element.isDirty
  );

  submitButton.disabled = !formValid;
};

export const handleInputForm = (formElements) => (e) => {
  const { id } = e.target;

  const targetElement = formElements[id];
  targetElement.isDirty = true;
  targetElement.validateAndUpdateUI();

  updateSubmitButtonState(formElements);
};

// --------login.js------
import { email, password, updateSubmitButtonState, handleInputForm } from './common/members.js'

const formElements = {
  email,
  password,
};

const handleInputEvent = handleInputForm(formElements);

const $form = document.querySelector("form");
$form.addEventListener("input", handleInputEvent);
```;

@dongqui dongqui merged commit d9670cd into codeit-bootcamp-frontend:Basic-이수정 Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants